Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Get example working with meshtext #22

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

TotalKrill
Copy link

@TotalKrill TotalKrill commented Dec 16, 2022

This affects the following issues: #11 , #18 , #20

Basically, quite straight forward to use meshtext, performance seems to take a large hit, and the most naive implementation I could try does not take into regard any of the width, height or similar settings, making all text the same size, unless u use bevys internal scale system. But this works in wasm.

To get around some lifetime checks, this implementation leaks memory with Vec::leak() calls.

This implementation caches each mesh inside meshtexts built in caching, and not in bevy world, or bevy_text_mesh caching methods. The meshes are thus quite expensive to generate inside bevy, it could probably be faster to spawn each character mesh into bevy, and then assemble them one by one into a string or a smarter caching system

I will not have time to polish this, but I will leave it here, in case anyone feels like adding some features, or basing a better implementation on top of this, simple fixes and suggestions are very welcome on my branch, and will probably be merged without much hassle. Or just fork this and improve.

But many thanks to @blaind and @FrankenApps for creating these two awesome crates, and sharing them with the world so that we can contribute and help each other.

At least this will be usable to spawn static text (with some scaling trial and error) onto the world, and getting it to run in wasm.

@TotalKrill
Copy link
Author

Just realized I ran everything in debug mode, when running in release mode, its actually usable and quite performant (albeit looks ugly since I did not implement the Sizing and such)

@FrankenApps
Copy link

Crazy that this is even possible with such a small amount of changes.
Thought I'd share my 5 cents...

Yeah it would probably be better to just disable the built-in cache and instead create a cache with bevy meshes.
I guess the API of meshtext does not offer a lot of custom stuff (like bold, italic and font size) in general and I feel like an optimizer which reduces triangle count of the meshes is definitely needed to make rendering large amounts of text in a scene possible (as far as I am aware ttf2mesh does support this).

The biggest selling point at the moment would probably be wasm support and easier compilation (no C dependencies), but ttf2mesh surely is more mature and battle tested. The memory leak probably blocks this PR though. However using meshtext it would be possible in general to get rid of the current memory leak (e.g. the cache which is never emptied).

P.S. Didn't know of that library initially (someone mentioned it on discord some time ago). Seems pretty cool how it integrates with bevy. I had something like this in mind on top of meshtext originally but I decided to go with pure wgpu in the meantime.

@blaind
Copy link
Owner

blaind commented Jan 2, 2023

Thank you for the pull request, looks really good!

Ideally, the memory leak would be mitigated by not taking a 'static in Meshgenerator::new, but I think its doable at this crate as well.

For performance testing, there's an example at https://github.com/blaind/bevy_text_mesh/blob/main/examples/performance.rs. Provides visual cues only though, not console-printed metrics.

I wonder if it would be a good idea to feature-gate the backend, maybe with a shared trait and implementation for both. Higher performance with ttf2mesh, and more interoperability with meshtext. Eventually (or already?) meshtext could be the default option, though.

@TotalKrill
Copy link
Author

To be fair, I dont really think feature gating the backend is very useful, seems like a bit to much extra work, and FFI is basically to add a lot of extra complexity. For quite a small gain in this case.

Rather I would look at getting the features of this crate has a whole working with meshtext. There is probably a lot of caching that could be done to not have to rasterize/tesselate the font more than once per font and startup.

And to rely on the scaling inside bevy for changing sizes of meshes seems to me to be quite a good way to go. Im not very versed in what scaling of the meshes means, but from my limited understanding. Meshes are transfered to the GPU memory, and then the GPU is used for scaling them, meaning that its quite performant to load meshes once, and then have the GPU do operations on them. If someone could verify this, that would be a suggested solution.

@rparrett
Copy link

rparrett commented Jan 5, 2023

While working on removing that 'static, I stumbled on this fork: https://github.com/rkanati/trianglyph which seems to have a number of improvements, including that one. Requires a nightly compiler though, unfortunately.

@rparrett
Copy link

rparrett commented Jan 5, 2023

Ideally, the memory leak would be mitigated by not taking a 'static in Meshgenerator::new, but I think its doable at this crate as well.

Could you elaborate on that? I'm not sure how to get around bevy Resources requiring 'static as well.

Here's a branch of meshtext that uses owned_ttf_parser: https://github.com/rparrett/meshtext/tree/owned which seems to work...

@FrankenApps
Copy link

@rparrett Cool, I feel like introducing an owned feature in meshtext shouldn't require much effort and if it helps with that I could easily do this.
Asking myself if including the font through bevy's asset system is actually the preferred option here, though?

@rparrett
Copy link

rparrett commented Jan 5, 2023

I think that ideally, a mesh generator would use a handle to an existing font asset. Users would probably still want to persist the generator in a Resource though, so I think using owned font data is still important.

@FrankenApps
Copy link

@TotalKrill The latest release of meshtext introduces an owned feature which can be enabled to use owned_ttf_parser instead. Using this it should be possible to work around the memory issue.

@TotalKrill
Copy link
Author

Will have a look either later this week or the next. Might get some time to make this better, since now we might have a use for this crate, so it got bumped up a bit :)

@vandie
Copy link

vandie commented Dec 26, 2023

Just stumbled across this and saw that @FrankenApps had made a PR (TotalKrill#1) into this branch to resolve the ownership issue back in Jan but that since then nothing else had occurred. Is this still a consideration?

@TotalKrill
Copy link
Author

TotalKrill commented Dec 27, 2023

Just forgot about this, since I am not using it.

Anyway I just rebased it, and merged the changes from @FrankenApps . Something doesnt look right when running in web, but I didnt investigate, but it just might be that I didnt change the sizes through scaling.

If anyone needs this, this is quite far along, and now up to date with 0.12

@TotalKrill TotalKrill changed the title [wip] [Do not merge] Get example working with meshtext [wip] Get example working with meshtext Dec 27, 2023
@TotalKrill TotalKrill changed the title [wip] Get example working with meshtext Get example working with meshtext Dec 27, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants